Use rust's own bool type in abstraction crate #61
Conversation
vkkoskie
force-pushed
the
use-primitive-bool
branch
from
97182bd
to
b6c62ee
Compare
|
Ref for the data layout, which is also present in a comment in the commit: https://doc.rust-lang.org/reference/type-layout.html#primitive-data-layout |
vkkoskie
force-pushed
the
use-primitive-bool
branch
from
b6c62ee
to
d2a795a
Compare
The Bbool type was translating between the FFI's CK_BBOOL and rust's own bool, by providing a backing intermediate type. But the storage for a bool is guaranteed to be large enough for the job, and the point of conversion from byte back to bool is already structured to check for validity. Signed-off-by: Keith Koskie <[email protected]>
vkkoskie
force-pushed
the
use-primitive-bool
branch
from
d2a795a
to
1b34cc0
Compare
| @@ -617,7 +617,7 @@ impl Attribute { | |||
| | Attribute::Verify(_) | |||
| | Attribute::VerifyRecover(_) | |||
| | Attribute::Wrap(_) | |||
| | Attribute::WrapWithTrusted(_) => 1, | |||
| | Attribute::WrapWithTrusted(_) => std::mem::size_of::<bool>(), | |||
There was a problem hiding this comment.
Extreme corner case -> CK_BBOOL is defined as an unsigned char. What if the architecture defines a char to not be 8-bits (e.g. not POSIX compliant).
Googling this question turns up two TI DSPs that if someone implemented a cryptoki token on I'd question their sanity.
I think this is fine, but just wanted to pose the question.
There was a problem hiding this comment.
Good catch. It may be possible at an architectural level, but the standard itself excludes that possibility by explicitly saying "unsigned 8-bit value".
At this particular location, the type being sized is exactly the type that the raw pointer points to. So if, say, CK_BBOOL somehow gets bigger because of a weird backing c_uchar, the ptr and len calls still match each other. And the provider should return CKR_BUFFER_TOO_SMALL or some other error code.
|
This looks fine by me. And since Rust's |
Funny enough, there is some ambiguity here. CK_TRUE is defined to 1, but the range of valid values is much more lax:
Should the range error condition on the u8->bool be removed? |
That's why I love this spec 🙄
Yeah probably to be as spec-compliant as possible (which I have droned on about in the past). |
Signed-off-by: Keith Koskie <[email protected]>
Head branch was pushed to by a user without write access
If it's any consolation, I don't like it either. |
The Bbool type was translating between the FFI's CK_BBOOL and rust's own bool, by providing a backing intermediate type. But the storage for a bool is guaranteed to be large enough for the job, and the point of conversion from byte back to bool is already structured to check for validity.